-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: properly handle Raises section for GoogleDocstring #56
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments.
docfx_yaml/extension.py
Outdated
for PATTERN in [REF_PATTERN, REF_PATTERN_LAST]: | ||
lines = _resolve_reference_in_module_summary(PATTERN, lines) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop the for loop and call the function directly twice? I was confused at first with what was happening here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. I'll add comments as well.
tests/test_unit.py
Outdated
PATTERNS = [REF_PATTERN, REF_PATTERN_LAST] | ||
for PATTERN in PATTERNS: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the Python style to use all caps for references to constants? Would expect this to be pattern
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
references shouldn't be. Done.
@@ -73,6 +73,7 @@ class Bcolors: | |||
REFFUNCTION = 'func' | |||
INITPY = '__init__.py' | |||
REF_PATTERN = ':(py:)?(func|class|meth|mod|ref|attr|exc):`~?[a-zA-Z0-9_\.<> ]*?`' | |||
REF_PATTERN_LAST = '~(([a-zA-Z0-9_<>]*\.)*[a-zA-Z0-9_<>]*)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment saying what this is for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
docfx_yaml/extension.py
Outdated
@@ -184,23 +187,31 @@ def _refact_example_in_module_summary(lines): | |||
return new_lines | |||
|
|||
|
|||
def _resolve_reference_in_module_summary(lines): | |||
def _resolve_reference_in_module_summary(PATTERN, lines): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same PATTERN
vs pattern
comment from before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
Before you open a pull request, note that this repository is forked from here.
Unless the issue you're trying to solve is unique to this specific repository,
please file an issue and/or send changes upstream to the original as well.
When there's special characters in
Raises section for type,
GoogleDocstring
does not handle it well and crashes. To avoid this, and to also have it properly be handled byGoogleDocstring
, I've decided to strip all occurrences of~type
and<xref:type>
entries from special characters before runningGoogleDocstring
.Added a unit test to verify that the strings get parsed properly.
Since this touches on the same function
_extract_docstring_info
as #55, I'll be adding unittest for the function's portion after it's merged and I've resolved git merge conflicts.Fixes #53